Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

const: Add const keyword for all name pointer parameters #414

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MaJerle
Copy link

@MaJerle MaJerle commented Oct 2, 2024

This pull requests fixes the compiler warning when creating TX objects with literal strings since we are passing literal string to the function accepting CHAR *, that can, by design, modify input parameter and therefore cause undefined system behavior.

All name pointers become const CHAR* type.

There is no backward incompatibility when creating objects, but there is a potential backward in-compatibility when calling tx_<modulename>_name_get function, because parameter shall be changed to const char* variable.

Currently we can get the name as:

CHAR* name;
tx_thread_name_get(..., &name);

New required function call:

const CHAR* name;
tx_thread_name_get(..., &name);

I've not seen tests for getting the name in the threadx, I also clearly assume that people using _get api are the minority.
From safety and quality standpoint, this upgrade makes a lot of sense.

PR checklist

  • Updated function header with a short description and version number
  • Added test case for bug fix or new feature
  • Validated on real hardware C-M4, GCC

@fdesbiens
Copy link
Contributor

@eclipse-threadx/iot-threadx-committers I need one of you to review this PR, please.

@fdesbiens
Copy link
Contributor

Thank you for this contribution, @MaJerle. I asked the group of committers to review it. I appreciate your patience.

Before we can merge this PR, you need to sign the Eclipse Contributor Agreement (ECA). The purpose of the ECA is to provide a written record that you have agreed to provide your code and documentation contributions under the licenses used by the Eclipse ThreadX project. It also makes it clear that you are promising that what you are contributing to Eclipse is code you wrote, and you have the necessary rights to contribute it to our projects. And finally, it documents a commitment from you that your open source contributions will be permanently on the public record.

Signing the ECA requires an Eclipse Foundation account if you do not already have one. You can create one for free at https://accounts.eclipse.org.

Be sure to use the same email address when you register for the account that you intend to use on Git commit records.

@billlamiework
Copy link

In principle, I agree with this request, and we should have included this in the original APIs. However, now that the API has been public for 28 years, I would refrain from making this change. I don't like anything that alters the published API. That said, if we were to do this, we would also have to update the user guide and, I think, the name pointer storage in all the control blocks, e.g., TX_THREAD, TX_QUEUE, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants